Skip to content

[PowerX] use the configurable AMD exporter and native power telemetry - #3781

Merged
edwingao28 merged 12 commits into
mainfrom
feat/powerx-amd-upstream-572
Oct 14, 2026
Merged

edwingao28 merged 12 commits into
mainfrom
feat/powerx-amd-upstream-572

Conversation

@edwingao28

@edwingao28 edwingao28 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Enable native srt-slurm GPU power collection for single-node fixed-sequence and AgentX jobs, including AMD's digest-pinned high-resolution exporter. Publish the native power package and result-side validation report, honor required-power failures, accept temperature samples, and reject packages whose producer recorded failed clock synchronization.

Rebased onto main after Alec's recipe-refactor stack merged. Native telemetry is composed through a shared fragment and bound after variant selection; exporter settings and workspace mounts come from cluster records. Recipe generation validates against each eligible cluster's exporter defaults. Eval-only jobs disable telemetry, and existing multi-node power policies remain in place.

Retains NVIDIA/srt-slurm#573 as a local patch to restrict worker-node samples to the job's participating GPUs. Includes the cleanup previously merged through #3798.

Replayed five saved hardware points through the refactored consumers: MI355X fixed-sequence and AgentX, plus B200 TP2/TP4 fixed-sequence and AgentX. All passed strict validation with every published power metric identical to the saved hardware results and original PR implementation.

Smoke runs on d357d8b3b4:

Lane Config key Run Result
MI355X fixed-seq TP4 c4 qwen3.5-fp8-mi355x-sglang 38520801937 pass
MI355X AgentX TP4 c4 (fast) qwen3.5-fp8-mi355x-sglang-agentic-mtp 38520804956 pass
B200 fixed-seq TP4 + TP2 c4 qwen3.5-fp4-b200-sglang 38520807831 pass
B200 AgentX TP4 c10 (fast) minimaxm3-fp4-b200-trtllm-agentic-mtp 38522822457 pass

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution!

  • Review: If this PR changes files owned by someone other than a repository admin or @SemiAnalysisAI/core, ask one eligible CODEOWNER to complete the latest PR_REVIEW_CHECKLIST.md before contacting a core maintainer on Slack. Follow the template exactly, including As a PR reviewer and CODEOWNER, I have reviewed this and have, so sign-off verification triggers.
  • PR verification: Sweeps run only when the PR appends an inferencex-e2e/perf-changelog.yaml entry and carries exactly one primary label: full-sweep-fail-fast (strongly recommended; canary plus per-matrix fail-fast), full-sweep-enabled (canary; matrix jobs continue after a failure), or non-canary-full-sweep-enabled (no canary or fail-fast). The modifiers all-evals, evals-only, and agentx-fast require a primary label. On fork PRs, a maintainer applies the label. See sweep labels and reuse.
  • After merging: PR authors must ensure all GitHub Actions jobs pass. Transient failures often pass on rerun; see how to rerun failed jobs.
中文

感谢你的贡献!

  • **审阅:**如果 PR 修改的文件归属于仓库管理员及 @SemiAnalysisAI/core 之外的 CODEOWNER,请先联系一位有资格的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,再通过 Slack 联系核心维护者。必须严格遵循模板,并保留 As a PR reviewer and CODEOWNER, I have reviewed this and have,才能触发签核验证。
  • **PR 验证:**只有当 PR 在 inferencex-e2e/perf-changelog.yaml 末尾追加了条目,并且恰好带有一个主标签时,才会运行扫描:full-sweep-fail-fast(强烈推荐;canary 加逐矩阵 fail-fast)、full-sweep-enabled(有 canary;矩阵任务在失败后继续运行)或 non-canary-full-sweep-enabled(无 canary,也无 fail-fast)。修饰标签 all-evals、evals-only 和 agentx-fast 必须与主标签一起使用。fork PR 的标签由维护者添加。参见扫描标签与复用。
  • **合并后:**PR 作者必须确保所有 GitHub Actions 任务通过。临时性失败通常可以通过重新运行恢复;参见重新运行失败任务的说明。

@edwingao28
edwingao28 marked this pull request as ready for review October 7, 2026 17:43
@edwingao28
edwingao28 requested a review from a team October 7, 2026 17:43
@edwingao28
edwingao28 requested a review from cquil11 as a code owner October 7, 2026 17:43
@claude

claude Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 1m 49s —— View job


PR #3781 review: one blocking changelog issue

  • Read the diff against origin/main
  • Reviewed the launch and driver changes (srt/__init__.py, collect.py, config.py, single_node.py)
  • Reviewed the power result consumers (power_adapter.py, fixed_sequence.py, multinode.py, agentic run.py, fixed_seq.py)
  • Checked the perf-changelog entries
  • Ran the touched tests: not done, because shell commands in this job need approval

🔴 Blocking: inferencex-e2e/perf-changelog.yaml gets five new blocks, but the rule from #3788 is one block per PR. One block is for #3610, which merged without this entry. Three are for #3684/#3685, which this PR supersedes, and they describe states it replaced: the AMD profile patch, the prepared-exporter staging, and "Requires … before rollout". This PR's own block still says "Hardware qualification pending", even though the PR body reports MI355X and B200 qualification passed. The inline comment has a suggestion that merges these into one #3781 block.

The launch and power code had no blocking issues:

  • Exporter fallback: this PR removes the built-in DCGM exporter image, but every srt-slurm cluster in runners.yaml defines default_gpu_exporter, so stage_gpu_exporter will find one.
  • Image staging: the AMD exporter is a public image pinned by digest and is staged through backend.stage_image, as expected.
  • Single-node power:
    • POWER_ARTIFACT_DIR reaches process_result through GITHUB_ENV.
    • GPU_COUNT is exported on the single-node path, so aggregate_power_result can read it.
    • Eval-only points skip both telemetry and the SRT_MEASUREMENT_WINDOW_DIR requirement.
  • Agentic power switches: run.py now requires ENABLE_AGENTX_POWER and REQUIRE_POWER for agentic points. Single-node agentic points only launch through srt-slurm, which sets both, so nothing else breaks.

One non-blocking note: the multinode eval upload no longer includes agent_preds.json, predictions.jsonl, swebench_report_*.json or *.traj*, but the retry uploads still list them. Those files belong to the retired SWE-bench lane, so this is harmless.


- config-keys:
- "*"
description:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one perf changelog entry as discussed earlier

Comment thread inferencex-e2e/perf-changelog.yaml Outdated
@cquil11

cquil11 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

moving srt slurm submodule bump to separate pr and rebasing this one

@cquil11
cquil11 force-pushed the feat/powerx-amd-upstream-572 branch from 4a64d00 to 4172b89 Compare October 7, 2026 17:54
@cquil11
cquil11 force-pushed the feat/powerx-amd-upstream-572 branch 4 times, most recently from 4a64d00 to 4172b89 Compare October 7, 2026 17:54
@cquil11 cquil11 changed the title [PowerX] run merged srt-slurm #572 with the fixed AMD exporter / [PowerX] 使用修复后的 AMD exporter 运行已合并的 srt-slurm #572 [PowerX] use the configurable AMD exporter and native power telemetry Oct 7, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond the inline finding, I also checked the dropped matrix.config['require-power'] fallback for test-sweep-single-node (e2e-tests.yml:509): single-node fixed-sequence recipes are validated to reject a per-recipe require-power field (infx/matrix/generate.py rejects it outside the one scoped multinode fixed-sequence), so only the global inputs.require-power flag can ever apply there — the missing fallback is not a behavior change.

Extended reasoning...

The PR re-pins srt-slurm to an upstream producer, reworks single-node power staging/validation in collect.py, and touches several CI workflow wiring points for require-power propagation; no auth/crypto surface is involved. A confirmed finding (collect.py:76) about a new recursive copytree that can turn log-staging I/O errors into benchmark-run failures is being posted inline, so approval is not warranted regardless. I independently verified one of the ruled-out candidates (the single-node require-power fallback removal) against infx/matrix/generate.py's validation, which rejects a per-recipe require-power flag for single-node fixed-sequence configs, confirming it is a non-issue.

Comment thread inferencex-e2e/infx/launch/drivers/srt/collect.py Outdated
Comment thread .github/workflows/benchmark-multinode-tmpl.yml
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

inferencex-e2e/infx diff: +739 -262

Generally most InferenceX submissions don't need to add changes to the infx package. On the off chance they do, generally the diff is small. Please prompt your agent to clean your PR to keep your diff to only necessary & minimal changes too. We have an increased code quality bar for changes to the infx package.

中文

大多数 InferenceX 提交通常不需要修改 infx 包。即使确实需要,改动通常也很小。请让你的 agent 清理 PR,使改动只包含必要且最小的内容。infx 包的改动有更高的代码质量要求。

@cquil11
cquil11 force-pushed the feat/powerx-amd-upstream-572 branch from a88e001 to 6dc48b1 Compare October 14, 2026 04:56
cquil11 and others added 10 commits October 14, 2026 00:10
中文:将主分支合并到 AMD 功耗遥测 PR。
The launcher no longer re-reads the bound recipe, patches its request, or
writes $GITHUB_ENV. REQUIRE_POWER alone sets telemetry.required, so a
single-node fragment may not set it, and result processing takes the
producer SHA from the launcher stamp as the multi-node workflow does.
select_recipe already omits the native telemetry block for eval-only points,
so the binder no longer rewrites telemetry.enabled.
Single-node and multi-node AgentX run the adapter through one helper, and the
single-node adapter writes its sidecar into the workspace instead of the
launcher copying it there.
The aggregate already records num_gpus from the same environment as
GPU_COUNT, so the adapter drops --expected-num-gpus.
@edwingao28
edwingao28 merged commit 6fff051 into main Oct 14, 2026
73 of 75 checks passed
@edwingao28
edwingao28 deleted the feat/powerx-amd-upstream-572 branch October 14, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants